Skip to content

[do-not-mirror][do-not-merge] chore(fork): restore the shadowed round-four regression tests - #21

Closed
jdsika wants to merge 5 commits into
integration/3.0from
chore/restore-shadowed-tests
Closed

jdsika wants to merge 5 commits into
integration/3.0from
chore/restore-shadowed-tests

Conversation

@jdsika

@jdsika jdsika commented Nov 11, 2026

Copy link
Copy Markdown

What

Rename the second TestRoundFourRegressions class in .github/fork/test_forkctl.py to
TestRoundFiveRegressions.

Why

The git-identity gate fix added a new regression class and reused a name that already existed at
line 1286. Python rebinds the name at class-definition time, so the first class object was
discarded before unittest ever collected it, and its six tests stopped running — silently, with
a green suite. ruff's F811 caught it on #14; the unit suite could not, because the symptom is
tests that are absent rather than failing.

How tested

CI, plus the full forkctl suite locally:

  • python test_forkctl.py → Ran 133 tests, OK (584s). Previously 127 ran while 133 existed;
    the six recovered tests are exactly TestRoundFourRegressions.
  • python .github/fork/forkctl.py check-commit --chore → OK, track B
  • pre-commit run --all-files → all 4 hooks passed

New behaviour is covered by a new test: not applicable — this restores six existing tests to the
collected set. The proof is the collected count, which is asserted above and visible in CI.

Upstream disposition

  • Mirror PR requested now — a human must verify title, body and diff first, and the body
    carries the disclosure from .github/fork/PROCESS.md → Agent authorship
  • Contributable, held for now because:
  • Never upstream — fork-only chore(fork): change

Risk / notes

Fold this in, do not merge it.

Worth noting what this says about the guard set: a duplicate class name is a defect that makes
tests disappear, which no passing test suite can report. ruff lint (#14) is the only thing in
the toolchain that sees it, which is a concrete argument for landing #14 rather than treating it
as a style change.

ASCS-eV/PufferDrive is consumed as a git submodule internally while feeding
features back to Emerge-Lab/PufferDrive. This commit carries everything that
exists only in the fork, and is the single place fork-only content lives.

Branch model:
  3.0              verbatim, fast-forward-only mirror of upstream/3.0
  integration/3.0  default branch; this commit plus a linear stack of
                   single-commit features; what internal consumers pin
  feature/<slug>   one feature, exactly one commit
  mirror/<slug>    cherry-picked onto pure upstream/3.0 for upstream PRs

integration/3.0 is the default branch because GitHub reads the PR template,
the contributing guidelines, the agent instructions and scheduled workflows
only from the default branch, and none of those files may exist on 3.0
without destroying the mirror. A clone checks out the default branch, so the
copy of the instructions here is the process as far as every agent and every
submodule consumer is concerned; a rule that lives only in an open PR has no
effect.

Nothing reaches integration/3.0 until three gates pass:
  1. CI green on the final commit.
  2. Adversarial review by someone other than the author, with the findings
     written down -- including the probes that found nothing.
  3. Upstream-readiness proven rather than asserted: the commit cherry-picks
     onto a branch cut from pure upstream/3.0 and produces an upstream-clean
     diff. We stack only commits we could contribute today.

chore(fork): commits are exempt from gate 3, since fork-only infrastructure
can never be upstream-ready. They carry two orthogonal markers: do-not-mirror
(must never reach upstream) and do-not-merge (the merge button is never
pressed -- merging would create a second chore(fork): commit and break the
single-commit invariant that every sync asserts). Such a PR is folded into
this commit by fixup and closed unmerged.

Contents:
- .github/copilot-instructions.md: normative process for coding agents
- CONTRIBUTING.md: human orientation, same rules
- .github/pull_request_template.md: testing and stack-hygiene gates
- .github/workflows/sync-mirror.yml: fast-forward-only mirror sync that
  refuses any non-fast-forward instead of rewriting the upstream base
- AGENTS.md / CLAUDE.md: fenced pointer so agents find the fork process
  without colliding with upstream-owned content
- ci.yml / install.yml: run CI for PRs targeting integration/3.0, which
  upstream's branch filters would otherwise skip

A standing draft PR from integration/3.0 into 3.0 gives a continuous CI
verdict on the combined stack; it must never be merged.

Everything here is fork-only and must never be cherry-picked upstream.
The Linux CI jobs fail while installing the package:

    [Errno 122] Disk quota exceeded

Errno 122 is EDQUOT, hit while unpacking the CUDA torch stack that
`pip install -e .` resolves by default. torch alone is 554.6 MB and the
accompanying nvidia-* wheels add another ~2.4 GB, which no longer fits
the runner's quota. The failure occurs during "Installing build
dependencies", i.e. the isolated PEP 517 build environment is already
enough to exceed it on its own.

The trigger is a runner image change, not a code change:

  * upstream 11_bernhard_dev, 2026-10-20, Image: ubuntu-24.04 -> success
  * same workflows, 2026-10-23, Image: ubuntu-26.04 -> failure

Both resolve an identical wheel set (torch 2.14.1, 554.6 MB). The
`ubuntu-latest` label began migrating to Ubuntu 26 on 2026-10-19
(actions/runner-images#14748), between those two runs. Both workflows
already carry "Free up disk space" steps, so these jobs were running
close to the limit before the migration.

Fix: install CPU-only torch from download.pytorch.org, then build with
--no-build-isolation so the build reuses it instead of resolving torch
a second time. --no-build-isolation needs the build dependencies present
up front, hence the explicit setuptools/wheel/numpy install; the
`setuptools<81` bound matches the pin already in install_requires.

This loses no coverage: BUILD_CUDA_EXT keys off CUDA_HOME/ROCM_HOME,
which is unset on these runners, so the CUDA sources were never compiled
here. tests/smoke_tests/Dockerfile already installs torch this way.

macOS keeps the plain `pip install -e .` unchanged: its PyPI wheels are
already CPU-only, and leaving it alone keeps the default PEP 517
build-isolation path under test somewhere in the matrix.
The shipped puffer_drive.yaml sets policy.fused_slot_encoder=true, and
DriveBackbone raises ImportError when that is set without triton. Two CI
jobs build a policy straight from the shipped config, so Unit Tests and
Notebook Execution have been failing on every pull request:

  ImportError: fused_slot_encoder=true requires triton

The smoke and eval suites already pin the flag off for exactly this reason;
these two call sites were simply missed. test_action_selection only exercises
the discrete-to-continuous action table, which the fused kernel has no part
in, so it pins the flag off outright. The inference notebook keeps the kernel
whenever it has a CUDA device and drops it otherwise, which is also what a
reader running the notebook on a laptop needs.

Neither change touches the training path or the kernel itself: with triton
and a GPU present, both call sites behave exactly as before.
The two instruction files are byte-identical except for one line, and that line is a contradiction rather than a Claude-specific tweak. AGENTS.md asked for comments that explain invariants; CLAUDE.md banned them by default with a hard one-line cap and an explicit list of banned forms.

Agents load one file or the other depending on the tool, so the same codebase was being written to two different standards, and neither copy said the other existed. This adopts the stricter rule in both, because it is the one written for this codebase and the one the recent code follows.
The git-identity fix added a second class literally named
TestRoundFourRegressions. Python rebinds the name, so the original
class object was discarded before unittest could collect it and its
six tests stopped running silently.

Rename the newer class to TestRoundFiveRegressions. The suite collects
and runs all 133 methods again, up from 127.
@jdsika jdsika added do-not-mirror Fork-only change; must never be sent to Emerge-Lab/PufferDrive do-not-merge Never use the merge button; applied by fixup (3.3) or never merged at all labels Nov 11, 2026
@jdsika

jdsika commented Nov 11, 2026

Copy link
Copy Markdown
Author

Folded into the fork base, not merged, per PROCESS.md -> Changing fork-only content.

  • fork base: fffe8fd09 -> c509ccd52
  • integration/3.0: dafe1bd17 -> 74a648b67
  • fork-only/3.0: republished at c509ccd52

The product tree is byte-identical to dafe1bd17, which was green on all 13 checks, so the sim suites were not re-run; git diff --name-only dafe1bd17 74a648b67 returns only .github/fork/test_forkctl.py.

@jdsika jdsika closed this Nov 11, 2026
@jdsika
jdsika deleted the chore/restore-shadowed-tests branch November 11, 2026 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge Never use the merge button; applied by fixup (3.3) or never merged at all do-not-mirror Fork-only change; must never be sent to Emerge-Lab/PufferDrive

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant